Skip to content

Conversation

@kblaschke
Copy link
Member

There were mistakes on multiple levels:

  • The scanner returning the wrong operator symbols for & and |
  • The parser was not adding the correct functions to the tree
  • The implementations of the bitwise AND/OR operators (| and &) were completely missing

Already bumped the version to 1.0.2 in preparation of a new bugfix release.

Thanks to OfficialIncubo for finding this issue!

There were mistakes on multiple levels, e.g. the scanner returning the wrong operator symbols for & and |, the parser not adding the correct functions to the tree and the implementation of the bitwise AND/OR operators (| and &) were completely missing.
@kblaschke kblaschke self-assigned this Jun 30, 2025
@kblaschke kblaschke added the bug Something isn't working label Jun 30, 2025
Comment on lines 24 to 25
{ "/*or*/", prjm_eval_func_or, 2, true, false },
{ "/*and*/", prjm_eval_func_and, 2, true, false },
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the special "comment" syntax here as ns-eel2 doesn't have any internal name for these two operator implementations. Since the scanner will skip over comments, this text can never occur as a function token in the parser and thus doesn't influence the syntax in any way.

Copy link
Member

@revmischa revmischa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but or and bor aren't great names imo because the b could be bitwise or boolean...

@kblaschke
Copy link
Member Author

Looks good but or and bor aren't great names imo because the b could be bitwise or boolean...

Yeah, said this to Incubo as well, that's definitely one of the reasons this mixup occurred, as the naming was done by the Milkdrop authors and is even worse in the original code - especially as the implementations are pure assembly, so it's not overly obvious what the functions actually do. We could rename the internal implementations though, but the bor() function in the expression code needs to stay as-is, sadly.

I'll rename the functions so the code gets more readable.

@kblaschke kblaschke merged commit 84d25bc into projectM-visualizer:master Jul 2, 2025
5 checks passed
@kblaschke kblaschke deleted the fix-and-or-operators branch July 2, 2025 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants